Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add system metrics clarification around containers #2388

Closed

Conversation

trask
Copy link
Member

@trask trask commented Feb 28, 2022

The JVM Metrics working group would like to emit a couple key system metrics from inside the JVM (e.g. for users who aren't using the collector/agent/other mechanism), and we'd like to add clarification somewhere that these metrics should be interpreted as container-level metrics and not host-level metrics.

cc: @jonatan-ivanov @jack-berg @kittylyst

@trask trask requested review from a team February 28, 2022 22:01
Copy link
Member

@tigrannajaryan tigrannajaryan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This clarification matches my understanding.

@arminru arminru added area:semantic-conventions Related to semantic conventions spec:metrics Related to the specification/metrics directory labels Mar 1, 2022
@arminru
Copy link
Member

arminru commented Mar 1, 2022

Thanks for the clarification, this would also match my understanding.

Co-authored-by: Armin Ruech <armin.ruech@dynatrace.com>
Copy link
Member

@bogdandrutu bogdandrutu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I read few times description/change. I feel that the motivation for this change is wrong, the change itself is ok. I do believe that we should not offer this in the java instrumentation until we have lots of requests for this. This is one of the easiest way to make things inconsistent and then blame that java or collector or whatever does inconsistent things etc.

@jmacd
Copy link
Contributor

jmacd commented Mar 1, 2022

I believe the principle here that when a process wishes to observe metrics on behalf of the system it's running in, we're defining that "system" as the inner-most container.

I believe @bogdandrutu's concern is that we should not expect a process to observe metrics on behalf of the system it's running in because it leads to duplicated code, but that doesn't change the principle stated above. If there is not another entity observing the system you are running in, you should feel free to do that yourself. 👍

@bogdandrutu
Copy link
Member

I believe @bogdandrutu's concern is that we should not expect a process to observe metrics on behalf of the system it's running in because it leads to duplicated code, but that doesn't change the principle stated above. If there is not another entity observing the system you are running in, you should feel free to do that yourself. 👍

@jmacd not only duplicate code, but also confusion. Imagine how confusing would be for a Java application owner, that uses the collector as well. What does system.* mean? Where do I enable them? Are they the same (probably not)? etc.

@trask
Copy link
Member Author

trask commented Mar 1, 2022

@bogdandrutu are you ok if these system.* metrics are opt-in from the Java Instrumentation side? there's definitely need for them, we cannot force everyone to run the collector

@bogdandrutu
Copy link
Member

there's definitely need for them, we cannot force everyone to run the collector

Prove me with numbers / requests for this. The "we cannot force them" can be translated "if you want system metrics" here is a small binary that does this for you. Otherwise you just create huge amount of work for all the language sigs.

@tigrannajaryan
Copy link
Member

I do believe that we should not offer this in the java instrumentation until we have lots of requests for this.

I didn't read the PR description carefully. I agree with what Bogdan wrote. I would definitely not expect that a Java application emits system metrics information (unless that Java application is purpose-built to be a system metric Collector).

@trask
Copy link
Member Author

trask commented Mar 2, 2022

I would definitely not expect that a Java application emits system metrics information (unless that Java application is purpose-built to be a system metric Collector).

For compatibility with the Java ecosystem (e.g. JFR, JMX, Micrometer) there are a couple of common "system" level metrics that are provided by the JVM that we would like to map to the OpenTelemetry world.

Can you clarify if the objection is specific to reusing the system.* namespace for these? And whether it is ok as long as we map these to something under process.runtime.jvm.*? (see #2292 for the proposal along that line)

@tigrannajaryan
Copy link
Member

Can you clarify if the objection is specific to reusing the system.* namespace for these? And whether it is ok as long as we map these to something under process.runtime.jvm.*? (see #2292 for the proposal along that line)

Yes. It looks surprising that the application emits metrics in the system.* namespace. process.runtime.jvm.* makes more sense to me, especially if it describes the CPUs available to the JVM and not all of the CPUs that the system has.

@trask
Copy link
Member Author

trask commented Mar 2, 2022

It looks surprising that the application emits metrics in the system.* namespace.

Thanks, I think this is the clarification we needed to move forward with JVM metrics specification.

@trask trask closed this Mar 2, 2022
@trask trask deleted the add-system-metrics-clarification branch March 2, 2022 18:25
@trask trask restored the add-system-metrics-clarification branch May 16, 2022 23:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:semantic-conventions Related to semantic conventions spec:metrics Related to the specification/metrics directory
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants